From: Dayllan Maza Date: Thu, 11 Apr 2019 19:54:10 +0000 (-0400) Subject: Rename BlockRestriction -> BlockRestrictionStore and wire it up as a service X-Git-Tag: 1.34.0-rc.0~1870^2 X-Git-Url: http://git.cyclocoop.org/%22.%24match%5B1%5D.%22?a=commitdiff_plain;h=de67ee197220132cb124809e214b088a99d974a5;p=lhc%2Fweb%2Fwiklou.git Rename BlockRestriction -> BlockRestrictionStore and wire it up as a service BlockRestriction was initially created as a static class and there is no reason why this shouldn't be available in the service container. Also renaming as BlockRestrictionStore to keep up with the new emerging naming patterns. Bug: T219684 Change-Id: If0b954f286d4759de2e3e41a0eb788e74bd72996 --- diff --git a/includes/Block.php b/includes/Block.php index c6b948239c..37b9ce521e 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -22,7 +22,7 @@ use Wikimedia\Rdbms\Database; use Wikimedia\Rdbms\IDatabase; -use MediaWiki\Block\BlockRestriction; +use MediaWiki\Block\BlockRestrictionStore; use MediaWiki\Block\Restriction\Restriction; use MediaWiki\Block\Restriction\NamespaceRestriction; use MediaWiki\Block\Restriction\PageRestriction; @@ -305,7 +305,9 @@ class Block { && $this->isSitewide() == $block->isSitewide() // Block::getRestrictions() may perform a database query, so keep it at // the end. - && BlockRestriction::equals( $this->getRestrictions(), $block->getRestrictions() ) + && $this->getBlockRestrictionStore()->equals( + $this->getRestrictions(), $block->getRestrictions() + ) ); } @@ -523,10 +525,10 @@ class Block { $dbw = wfGetDB( DB_MASTER ); - BlockRestriction::deleteByParentBlockId( $this->getId() ); + $this->getBlockRestrictionStore()->deleteByParentBlockId( $this->getId() ); $dbw->delete( 'ipblocks', [ 'ipb_parent_block_id' => $this->getId() ], __METHOD__ ); - BlockRestriction::deleteByBlockId( $this->getId() ); + $this->getBlockRestrictionStore()->deleteByBlockId( $this->getId() ); $dbw->delete( 'ipblocks', [ 'ipb_id' => $this->getId() ], __METHOD__ ); return $dbw->affectedRows() > 0; @@ -565,7 +567,7 @@ class Block { if ( $affected ) { $this->setId( $dbw->insertId() ); if ( $this->restrictions ) { - BlockRestriction::insert( $this->restrictions ); + $this->getBlockRestrictionStore()->insert( $this->restrictions ); } } @@ -585,12 +587,12 @@ class Block { ); if ( $ids ) { $dbw->delete( 'ipblocks', [ 'ipb_id' => $ids ], __METHOD__ ); - BlockRestriction::deleteByBlockId( $ids ); + $this->getBlockRestrictionStore()->deleteByBlockId( $ids ); $dbw->insert( 'ipblocks', $row, __METHOD__, [ 'IGNORE' ] ); $affected = $dbw->affectedRows(); $this->setId( $dbw->insertId() ); if ( $this->restrictions ) { - BlockRestriction::insert( $this->restrictions ); + $this->getBlockRestrictionStore()->insert( $this->restrictions ); } } } @@ -634,9 +636,9 @@ class Block { if ( $this->restrictions !== null ) { // An empty array should remove all of the restrictions. if ( empty( $this->restrictions ) ) { - $success = BlockRestriction::deleteByBlockId( $this->getId() ); + $success = $this->getBlockRestrictionStore()->deleteByBlockId( $this->getId() ); } else { - $success = BlockRestriction::update( $this->restrictions ); + $success = $this->getBlockRestrictionStore()->update( $this->restrictions ); } // Update the result. The first false is the result, otherwise, true. $result = $result && $success; @@ -653,11 +655,11 @@ class Block { // Only update the restrictions if they have been modified. if ( $this->restrictions !== null ) { - BlockRestriction::updateByParentBlockId( $this->getId(), $this->restrictions ); + $this->getBlockRestrictionStore()->updateByParentBlockId( $this->getId(), $this->restrictions ); } } else { // autoblock no longer required, delete corresponding autoblock(s) - BlockRestriction::deleteByParentBlockId( $this->getId() ); + $this->getBlockRestrictionStore()->deleteByParentBlockId( $this->getId() ); $dbw->delete( 'ipblocks', [ 'ipb_parent_block_id' => $this->getId() ], @@ -1069,7 +1071,9 @@ class Block { $this->mId = (int)$blockId; if ( is_array( $this->restrictions ) ) { - $this->restrictions = BlockRestriction::setBlockId( $blockId, $this->restrictions ); + $this->restrictions = $this->getBlockRestrictionStore()->setBlockId( + $blockId, $this->restrictions + ); } return $this; @@ -1367,7 +1371,9 @@ class Block { $fname ); if ( $ids ) { - BlockRestriction::deleteByBlockId( $ids ); + $blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); + $blockRestrictionStore->deleteByBlockId( $ids ); + $dbw->delete( 'ipblocks', [ 'ipb_id' => $ids ], $fname ); } } @@ -1941,7 +1947,7 @@ class Block { if ( !$this->mId ) { return []; } - $this->restrictions = BlockRestriction::loadByBlockId( $this->mId ); + $this->restrictions = $this->getBlockRestrictionStore()->loadByBlockId( $this->mId ); } return $this->restrictions; @@ -2163,4 +2169,12 @@ class Block { } } + /** + * Get a BlockRestrictionStore instance + * + * @return BlockRestrictionStore + */ + private function getBlockRestrictionStore() : BlockRestrictionStore { + return MediaWikiServices::getInstance()->getBlockRestrictionStore(); + } } diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 8c60dc7e52..7a5f0269c8 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -13,6 +13,7 @@ use GlobalVarConfig; use Hooks; use IBufferingStatsdDataFactory; use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; +use MediaWiki\Block\BlockRestrictionStore; use MediaWiki\Http\HttpRequestFactory; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Preferences\PreferencesFactory; @@ -435,6 +436,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'BlobStoreFactory' ); } + /** + * @since 1.33 + * @return BlockRestrictionStore + */ + public function getBlockRestrictionStore() : BlockRestrictionStore { + return $this->getService( 'BlockRestrictionStore' ); + } + /** * Returns the Config object containing the bootstrap configuration. * Bootstrap configuration would typically include database credentials diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index a82feaa22f..b11e129c6a 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -39,6 +39,7 @@ use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; use MediaWiki\Auth\AuthManager; +use MediaWiki\Block\BlockRestrictionStore; use MediaWiki\Config\ConfigRepository; use MediaWiki\Interwiki\ClassicInterwikiLookup; use MediaWiki\Interwiki\InterwikiLookup; @@ -83,6 +84,12 @@ return [ ); }, + 'BlockRestrictionStore' => function ( MediaWikiServices $services ) : BlockRestrictionStore { + return new BlockRestrictionStore( + $services->getDBLoadBalancer() + ); + }, + 'CommentStore' => function ( MediaWikiServices $services ) : CommentStore { return new CommentStore( $services->getContentLanguage(), diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php index 8aff2aa6b4..5615f46213 100644 --- a/includes/api/ApiQueryBlocks.php +++ b/includes/api/ApiQueryBlocks.php @@ -21,7 +21,7 @@ */ use Wikimedia\Rdbms\IResultWrapper; -use MediaWiki\Block\BlockRestriction; +use MediaWiki\MediaWikiServices; /** * Query module to enumerate all user blocks @@ -292,7 +292,8 @@ class ApiQueryBlocks extends ApiQueryBase { } } - $restrictions = BlockRestriction::loadByBlockId( $partialIds ); + $blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); + $restrictions = $blockRestrictionStore->loadByBlockId( $partialIds ); $data = []; $keys = [ diff --git a/includes/block/BlockRestriction.php b/includes/block/BlockRestriction.php deleted file mode 100644 index 2e8752e450..0000000000 --- a/includes/block/BlockRestriction.php +++ /dev/null @@ -1,436 +0,0 @@ - PageRestriction::class, - NamespaceRestriction::TYPE_ID => NamespaceRestriction::class, - ]; - - /** - * Retrieves the restrictions from the database by block id. - * - * @since 1.33 - * @param int|array $blockId - * @param IDatabase|null $db - * @return Restriction[] - */ - public static function loadByBlockId( $blockId, IDatabase $db = null ) { - if ( $blockId === null || $blockId === [] ) { - return []; - } - - $db = $db ?: wfGetDB( DB_REPLICA ); - - $result = $db->select( - [ 'ipblocks_restrictions', 'page' ], - [ 'ir_ipb_id', 'ir_type', 'ir_value', 'page_namespace', 'page_title' ], - [ 'ir_ipb_id' => $blockId ], - __METHOD__, - [], - [ 'page' => [ 'LEFT JOIN', [ 'ir_type' => PageRestriction::TYPE_ID, 'ir_value=page_id' ] ] ] - ); - - return self::resultToRestrictions( $result ); - } - - /** - * Inserts the restrictions into the database. - * - * @since 1.33 - * @param Restriction[] $restrictions - * @return bool - */ - public static function insert( array $restrictions ) { - if ( !$restrictions ) { - return false; - } - - $rows = []; - foreach ( $restrictions as $restriction ) { - if ( !$restriction instanceof Restriction ) { - continue; - } - $rows[] = $restriction->toRow(); - } - - if ( !$rows ) { - return false; - } - - $dbw = wfGetDB( DB_MASTER ); - - $dbw->insert( - 'ipblocks_restrictions', - $rows, - __METHOD__, - [ 'IGNORE' ] - ); - - return true; - } - - /** - * Updates the list of restrictions. This method does not allow removing all - * of the restrictions. To do that, use ::deleteByBlockId(). - * - * @since 1.33 - * @param Restriction[] $restrictions - * @return bool - */ - public static function update( array $restrictions ) { - $dbw = wfGetDB( DB_MASTER ); - - $dbw->startAtomic( __METHOD__ ); - - // Organize the restrictions by blockid. - $restrictionList = self::restrictionsByBlockId( $restrictions ); - - // Load the existing restrictions and organize by block id. Any block ids - // that were passed into this function will be used to load all of the - // existing restrictions. This list might be the same, or may be completely - // different. - $existingList = []; - $blockIds = array_keys( $restrictionList ); - if ( !empty( $blockIds ) ) { - $result = $dbw->select( - [ 'ipblocks_restrictions' ], - [ 'ir_ipb_id', 'ir_type', 'ir_value' ], - [ 'ir_ipb_id' => $blockIds ], - __METHOD__, - [ 'FOR UPDATE' ] - ); - - $existingList = self::restrictionsByBlockId( - self::resultToRestrictions( $result ) - ); - } - - $result = true; - // Perform the actions on a per block-id basis. - foreach ( $restrictionList as $blockId => $blockRestrictions ) { - // Insert all of the restrictions first, ignoring ones that already exist. - $success = self::insert( $blockRestrictions ); - - // Update the result. The first false is the result, otherwise, true. - $result = $success && $result; - - $restrictionsToRemove = self::restrictionsToRemove( - $existingList[$blockId] ?? [], - $restrictions - ); - - if ( empty( $restrictionsToRemove ) ) { - continue; - } - - $success = self::delete( $restrictionsToRemove ); - - // Update the result. The first false is the result, otherwise, true. - $result = $success && $result; - } - - $dbw->endAtomic( __METHOD__ ); - - return $result; - } - - /** - * Updates the list of restrictions by parent id. - * - * @since 1.33 - * @param int $parentBlockId - * @param Restriction[] $restrictions - * @return bool - */ - public static function updateByParentBlockId( $parentBlockId, array $restrictions ) { - // If removing all of the restrictions, then just delete them all. - if ( empty( $restrictions ) ) { - return self::deleteByParentBlockId( $parentBlockId ); - } - - $parentBlockId = (int)$parentBlockId; - - $db = wfGetDB( DB_MASTER ); - - $db->startAtomic( __METHOD__ ); - - $blockIds = $db->selectFieldValues( - 'ipblocks', - 'ipb_id', - [ 'ipb_parent_block_id' => $parentBlockId ], - __METHOD__, - [ 'FOR UPDATE' ] - ); - - $result = true; - foreach ( $blockIds as $id ) { - $success = self::update( self::setBlockId( $id, $restrictions ) ); - // Update the result. The first false is the result, otherwise, true. - $result = $success && $result; - } - - $db->endAtomic( __METHOD__ ); - - return $result; - } - - /** - * Delete the restrictions. - * - * @since 1.33 - * @param Restriction[]|null $restrictions - * @throws MWException - * @return bool - */ - public static function delete( array $restrictions ) { - $dbw = wfGetDB( DB_MASTER ); - $result = true; - foreach ( $restrictions as $restriction ) { - if ( !$restriction instanceof Restriction ) { - continue; - } - - $success = $dbw->delete( - 'ipblocks_restrictions', - // The restriction row is made up of a compound primary key. Therefore, - // the row and the delete conditions are the same. - $restriction->toRow(), - __METHOD__ - ); - // Update the result. The first false is the result, otherwise, true. - $result = $success && $result; - } - - return $result; - } - - /** - * Delete the restrictions by Block ID. - * - * @since 1.33 - * @param int|array $blockId - * @throws MWException - * @return bool - */ - public static function deleteByBlockId( $blockId ) { - $dbw = wfGetDB( DB_MASTER ); - return $dbw->delete( - 'ipblocks_restrictions', - [ 'ir_ipb_id' => $blockId ], - __METHOD__ - ); - } - - /** - * Delete the restrictions by Parent Block ID. - * - * @since 1.33 - * @param int|array $parentBlockId - * @throws MWException - * @return bool - */ - public static function deleteByParentBlockId( $parentBlockId ) { - $dbw = wfGetDB( DB_MASTER ); - return $dbw->deleteJoin( - 'ipblocks_restrictions', - 'ipblocks', - 'ir_ipb_id', - 'ipb_id', - [ 'ipb_parent_block_id' => $parentBlockId ], - __METHOD__ - ); - } - - /** - * Checks if two arrays of Restrictions are effectively equal. This is a loose - * equality check as the restrictions do not have to contain the same block - * ids. - * - * @since 1.33 - * @param Restriction[] $a - * @param Restriction[] $b - * @return bool - */ - public static function equals( array $a, array $b ) { - $filter = function ( $restriction ) { - return $restriction instanceof Restriction; - }; - - // Ensure that every item in the array is a Restriction. This prevents a - // fatal error from calling Restriction::getHash if something in the array - // is not a restriction. - $a = array_filter( $a, $filter ); - $b = array_filter( $b, $filter ); - - $aCount = count( $a ); - $bCount = count( $b ); - - // If the count is different, then they are obviously a different set. - if ( $aCount !== $bCount ) { - return false; - } - - // If both sets contain no items, then they are the same set. - if ( $aCount === 0 && $bCount === 0 ) { - return true; - } - - $hasher = function ( $r ) { - return $r->getHash(); - }; - - $aHashes = array_map( $hasher, $a ); - $bHashes = array_map( $hasher, $b ); - - sort( $aHashes ); - sort( $bHashes ); - - return $aHashes === $bHashes; - } - - /** - * Set the blockId on a set of restrictions and return a new set. - * - * @since 1.33 - * @param int $blockId - * @param Restriction[] $restrictions - * @return Restriction[] - */ - public static function setBlockId( $blockId, array $restrictions ) { - $blockRestrictions = []; - - foreach ( $restrictions as $restriction ) { - if ( !$restriction instanceof Restriction ) { - continue; - } - - // Clone the restriction so any references to the current restriction are - // not suddenly changed to a different blockId. - $restriction = clone $restriction; - $restriction->setBlockId( $blockId ); - - $blockRestrictions[] = $restriction; - } - - return $blockRestrictions; - } - - /** - * Get the restrictions that should be removed, which are existing - * restrictions that are not in the new list of restrictions. - * - * @param Restriction[] $existing - * @param Restriction[] $new - * @return array - */ - private static function restrictionsToRemove( array $existing, array $new ) { - return array_filter( $existing, function ( $e ) use ( $new ) { - foreach ( $new as $restriction ) { - if ( !$restriction instanceof Restriction ) { - continue; - } - - if ( $restriction->equals( $e ) ) { - return false; - } - } - - return true; - } ); - } - - /** - * Converts an array of restrictions to an associative array of restrictions - * where the keys are the block ids. - * - * @param Restriction[] $restrictions - * @return array - */ - private static function restrictionsByBlockId( array $restrictions ) { - $blockRestrictions = []; - - foreach ( $restrictions as $restriction ) { - // Ensure that all of the items in the array are restrictions. - if ( !$restriction instanceof Restriction ) { - continue; - } - - if ( !isset( $blockRestrictions[$restriction->getBlockId()] ) ) { - $blockRestrictions[$restriction->getBlockId()] = []; - } - - $blockRestrictions[$restriction->getBlockId()][] = $restriction; - } - - return $blockRestrictions; - } - - /** - * Convert an Result Wrapper to an array of restrictions. - * - * @param IResultWrapper $result - * @return Restriction[] - */ - private static function resultToRestrictions( IResultWrapper $result ) { - $restrictions = []; - foreach ( $result as $row ) { - $restriction = self::rowToRestriction( $row ); - - if ( !$restriction ) { - continue; - } - - $restrictions[] = $restriction; - } - - return $restrictions; - } - - /** - * Convert a result row from the database into a restriction object. - * - * @param \stdClass $row - * @return Restriction|null - */ - private static function rowToRestriction( \stdClass $row ) { - if ( array_key_exists( (int)$row->ir_type, self::$types ) ) { - $class = self::$types[ (int)$row->ir_type ]; - return call_user_func( [ $class, 'newFromRow' ], $row ); - } - - return null; - } -} diff --git a/includes/block/BlockRestrictionStore.php b/includes/block/BlockRestrictionStore.php new file mode 100644 index 0000000000..d242c87e2d --- /dev/null +++ b/includes/block/BlockRestrictionStore.php @@ -0,0 +1,449 @@ + PageRestriction::class, + NamespaceRestriction::TYPE_ID => NamespaceRestriction::class, + ]; + + /** + * @var ILoadBalancer + */ + private $loadBalancer; + + /* + * @param LoadBalancer $loadBalancer load balancer for acquiring database connections + */ + public function __construct( ILoadBalancer $loadBalancer ) { + $this->loadBalancer = $loadBalancer; + } + + /** + * Retrieves the restrictions from the database by block id. + * + * @since 1.33 + * @param int|array $blockId + * @param IDatabase|null $db + * @return Restriction[] + */ + public function loadByBlockId( $blockId, IDatabase $db = null ) { + if ( $blockId === null || $blockId === [] ) { + return []; + } + + $db = $db ?: $this->loadBalancer->getConnection( DB_REPLICA ); + + $result = $db->select( + [ 'ipblocks_restrictions', 'page' ], + [ 'ir_ipb_id', 'ir_type', 'ir_value', 'page_namespace', 'page_title' ], + [ 'ir_ipb_id' => $blockId ], + __METHOD__, + [], + [ 'page' => [ 'LEFT JOIN', [ 'ir_type' => PageRestriction::TYPE_ID, 'ir_value=page_id' ] ] ] + ); + + return $this->resultToRestrictions( $result ); + } + + /** + * Inserts the restrictions into the database. + * + * @since 1.33 + * @param Restriction[] $restrictions + * @return bool + */ + public function insert( array $restrictions ) { + if ( !$restrictions ) { + return false; + } + + $rows = []; + foreach ( $restrictions as $restriction ) { + if ( !$restriction instanceof Restriction ) { + continue; + } + $rows[] = $restriction->toRow(); + } + + if ( !$rows ) { + return false; + } + + $dbw = $this->loadBalancer->getConnection( DB_MASTER ); + + $dbw->insert( + 'ipblocks_restrictions', + $rows, + __METHOD__, + [ 'IGNORE' ] + ); + + return true; + } + + /** + * Updates the list of restrictions. This method does not allow removing all + * of the restrictions. To do that, use ::deleteByBlockId(). + * + * @since 1.33 + * @param Restriction[] $restrictions + * @return bool + */ + public function update( array $restrictions ) { + $dbw = $this->loadBalancer->getConnection( DB_MASTER ); + + $dbw->startAtomic( __METHOD__ ); + + // Organize the restrictions by blockid. + $restrictionList = $this->restrictionsByBlockId( $restrictions ); + + // Load the existing restrictions and organize by block id. Any block ids + // that were passed into this function will be used to load all of the + // existing restrictions. This list might be the same, or may be completely + // different. + $existingList = []; + $blockIds = array_keys( $restrictionList ); + if ( !empty( $blockIds ) ) { + $result = $dbw->select( + [ 'ipblocks_restrictions' ], + [ 'ir_ipb_id', 'ir_type', 'ir_value' ], + [ 'ir_ipb_id' => $blockIds ], + __METHOD__, + [ 'FOR UPDATE' ] + ); + + $existingList = $this->restrictionsByBlockId( + $this->resultToRestrictions( $result ) + ); + } + + $result = true; + // Perform the actions on a per block-id basis. + foreach ( $restrictionList as $blockId => $blockRestrictions ) { + // Insert all of the restrictions first, ignoring ones that already exist. + $success = $this->insert( $blockRestrictions ); + + // Update the result. The first false is the result, otherwise, true. + $result = $success && $result; + + $restrictionsToRemove = $this->restrictionsToRemove( + $existingList[$blockId] ?? [], + $restrictions + ); + + if ( empty( $restrictionsToRemove ) ) { + continue; + } + + $success = $this->delete( $restrictionsToRemove ); + + // Update the result. The first false is the result, otherwise, true. + $result = $success && $result; + } + + $dbw->endAtomic( __METHOD__ ); + + return $result; + } + + /** + * Updates the list of restrictions by parent id. + * + * @since 1.33 + * @param int $parentBlockId + * @param Restriction[] $restrictions + * @return bool + */ + public function updateByParentBlockId( $parentBlockId, array $restrictions ) { + // If removing all of the restrictions, then just delete them all. + if ( empty( $restrictions ) ) { + return $this->deleteByParentBlockId( $parentBlockId ); + } + + $parentBlockId = (int)$parentBlockId; + + $db = $this->loadBalancer->getConnection( DB_MASTER ); + + $db->startAtomic( __METHOD__ ); + + $blockIds = $db->selectFieldValues( + 'ipblocks', + 'ipb_id', + [ 'ipb_parent_block_id' => $parentBlockId ], + __METHOD__, + [ 'FOR UPDATE' ] + ); + + $result = true; + foreach ( $blockIds as $id ) { + $success = $this->update( $this->setBlockId( $id, $restrictions ) ); + // Update the result. The first false is the result, otherwise, true. + $result = $success && $result; + } + + $db->endAtomic( __METHOD__ ); + + return $result; + } + + /** + * Delete the restrictions. + * + * @since 1.33 + * @param Restriction[]|null $restrictions + * @throws MWException + * @return bool + */ + public function delete( array $restrictions ) { + $dbw = $this->loadBalancer->getConnection( DB_MASTER ); + $result = true; + foreach ( $restrictions as $restriction ) { + if ( !$restriction instanceof Restriction ) { + continue; + } + + $success = $dbw->delete( + 'ipblocks_restrictions', + // The restriction row is made up of a compound primary key. Therefore, + // the row and the delete conditions are the same. + $restriction->toRow(), + __METHOD__ + ); + // Update the result. The first false is the result, otherwise, true. + $result = $success && $result; + } + + return $result; + } + + /** + * Delete the restrictions by Block ID. + * + * @since 1.33 + * @param int|array $blockId + * @throws MWException + * @return bool + */ + public function deleteByBlockId( $blockId ) { + $dbw = $this->loadBalancer->getConnection( DB_MASTER ); + return $dbw->delete( + 'ipblocks_restrictions', + [ 'ir_ipb_id' => $blockId ], + __METHOD__ + ); + } + + /** + * Delete the restrictions by Parent Block ID. + * + * @since 1.33 + * @param int|array $parentBlockId + * @throws MWException + * @return bool + */ + public function deleteByParentBlockId( $parentBlockId ) { + $dbw = $this->loadBalancer->getConnection( DB_MASTER ); + return $dbw->deleteJoin( + 'ipblocks_restrictions', + 'ipblocks', + 'ir_ipb_id', + 'ipb_id', + [ 'ipb_parent_block_id' => $parentBlockId ], + __METHOD__ + ); + } + + /** + * Checks if two arrays of Restrictions are effectively equal. This is a loose + * equality check as the restrictions do not have to contain the same block + * ids. + * + * @since 1.33 + * @param Restriction[] $a + * @param Restriction[] $b + * @return bool + */ + public function equals( array $a, array $b ) { + $filter = function ( $restriction ) { + return $restriction instanceof Restriction; + }; + + // Ensure that every item in the array is a Restriction. This prevents a + // fatal error from calling Restriction::getHash if something in the array + // is not a restriction. + $a = array_filter( $a, $filter ); + $b = array_filter( $b, $filter ); + + $aCount = count( $a ); + $bCount = count( $b ); + + // If the count is different, then they are obviously a different set. + if ( $aCount !== $bCount ) { + return false; + } + + // If both sets contain no items, then they are the same set. + if ( $aCount === 0 && $bCount === 0 ) { + return true; + } + + $hasher = function ( $r ) { + return $r->getHash(); + }; + + $aHashes = array_map( $hasher, $a ); + $bHashes = array_map( $hasher, $b ); + + sort( $aHashes ); + sort( $bHashes ); + + return $aHashes === $bHashes; + } + + /** + * Set the blockId on a set of restrictions and return a new set. + * + * @since 1.33 + * @param int $blockId + * @param Restriction[] $restrictions + * @return Restriction[] + */ + public function setBlockId( $blockId, array $restrictions ) { + $blockRestrictions = []; + + foreach ( $restrictions as $restriction ) { + if ( !$restriction instanceof Restriction ) { + continue; + } + + // Clone the restriction so any references to the current restriction are + // not suddenly changed to a different blockId. + $restriction = clone $restriction; + $restriction->setBlockId( $blockId ); + + $blockRestrictions[] = $restriction; + } + + return $blockRestrictions; + } + + /** + * Get the restrictions that should be removed, which are existing + * restrictions that are not in the new list of restrictions. + * + * @param Restriction[] $existing + * @param Restriction[] $new + * @return array + */ + private function restrictionsToRemove( array $existing, array $new ) { + return array_filter( $existing, function ( $e ) use ( $new ) { + foreach ( $new as $restriction ) { + if ( !$restriction instanceof Restriction ) { + continue; + } + + if ( $restriction->equals( $e ) ) { + return false; + } + } + + return true; + } ); + } + + /** + * Converts an array of restrictions to an associative array of restrictions + * where the keys are the block ids. + * + * @param Restriction[] $restrictions + * @return array + */ + private function restrictionsByBlockId( array $restrictions ) { + $blockRestrictions = []; + + foreach ( $restrictions as $restriction ) { + // Ensure that all of the items in the array are restrictions. + if ( !$restriction instanceof Restriction ) { + continue; + } + + if ( !isset( $blockRestrictions[$restriction->getBlockId()] ) ) { + $blockRestrictions[$restriction->getBlockId()] = []; + } + + $blockRestrictions[$restriction->getBlockId()][] = $restriction; + } + + return $blockRestrictions; + } + + /** + * Convert an Result Wrapper to an array of restrictions. + * + * @param IResultWrapper $result + * @return Restriction[] + */ + private function resultToRestrictions( IResultWrapper $result ) { + $restrictions = []; + foreach ( $result as $row ) { + $restriction = $this->rowToRestriction( $row ); + + if ( !$restriction ) { + continue; + } + + $restrictions[] = $restriction; + } + + return $restrictions; + } + + /** + * Convert a result row from the database into a restriction object. + * + * @param \stdClass $row + * @return Restriction|null + */ + private function rowToRestriction( \stdClass $row ) { + if ( array_key_exists( (int)$row->ir_type, $this->types ) ) { + $class = $this->types[ (int)$row->ir_type ]; + return call_user_func( [ $class, 'newFromRow' ], $row ); + } + + return null; + } +} diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index 155d6a48f6..0ff517efc4 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -21,9 +21,9 @@ * @ingroup SpecialPage */ -use MediaWiki\Block\BlockRestriction; use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\Block\Restriction\NamespaceRestriction; +use MediaWiki\MediaWikiServices; /** * A special page that allows users with 'block' right to block users from @@ -950,8 +950,9 @@ class SpecialBlock extends FormSpecialPage { $currentBlock->isSitewide( $block->isSitewide() ); // Set the block id of the restrictions. + $blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); $currentBlock->setRestrictions( - BlockRestriction::setBlockId( $currentBlock->getId(), $restrictions ) + $blockRestrictionStore->setBlockId( $currentBlock->getId(), $restrictions ) ); } diff --git a/includes/specials/pagers/BlockListPager.php b/includes/specials/pagers/BlockListPager.php index 71cf787504..d09b3451b9 100644 --- a/includes/specials/pagers/BlockListPager.php +++ b/includes/specials/pagers/BlockListPager.php @@ -22,7 +22,6 @@ /** * @ingroup Pager */ -use MediaWiki\Block\BlockRestriction; use MediaWiki\Block\Restriction\Restriction; use MediaWiki\Block\Restriction\PageRestriction; use MediaWiki\Block\Restriction\NamespaceRestriction; @@ -419,7 +418,8 @@ class BlockListPager extends TablePager { if ( $partialBlocks ) { // Mutations to the $row object are not persisted. The restrictions will // need be stored in a separate store. - $this->restrictions = BlockRestriction::loadByBlockId( $partialBlocks ); + $blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); + $this->restrictions = $blockRestrictionStore->loadByBlockId( $partialBlocks ); } $lb->execute(); diff --git a/tests/phpunit/includes/BlockTest.php b/tests/phpunit/includes/BlockTest.php index 7874688e7b..df3de4a336 100644 --- a/tests/phpunit/includes/BlockTest.php +++ b/tests/phpunit/includes/BlockTest.php @@ -1,8 +1,9 @@ getId(), $pageFoo->getId() ); $namespaceRestriction = new NamespaceRestriction( $block->getId(), NS_USER ); - BlockRestriction::insert( [ $pageRestriction, $namespaceRestriction ] ); + $this->getBlockRestrictionStore()->insert( [ $pageRestriction, $namespaceRestriction ] ); $this->assertTrue( $block->appliesToTitle( $pageFoo->getTitle() ) ); $this->assertFalse( $block->appliesToTitle( $pageBar->getTitle() ) ); @@ -673,7 +674,7 @@ class BlockTest extends MediaWikiLangTestCase { $block->getId(), $title->getArticleID() ); - BlockRestriction::insert( [ $pageRestriction ] ); + $this->getBlockRestrictionStore()->insert( [ $pageRestriction ] ); $this->assertTrue( $block->appliesToPage( $title->getArticleID() ) ); @@ -699,7 +700,7 @@ class BlockTest extends MediaWikiLangTestCase { $block->insert(); $namespaceRestriction = new NamespaceRestriction( $block->getId(), NS_MAIN ); - BlockRestriction::insert( [ $namespaceRestriction ] ); + $this->getBlockRestrictionStore()->insert( [ $namespaceRestriction ] ); $this->assertTrue( $block->appliesToNamespace( NS_MAIN ) ); $this->assertFalse( $block->appliesToNamespace( NS_USER ) ); @@ -718,4 +719,12 @@ class BlockTest extends MediaWikiLangTestCase { $this->assertFalse( $block->appliesToRight( 'purge' ) ); } + /** + * Get an instance of BlockRestrictionStore + * + * @return BlockRestrictionStore + */ + protected function getBlockRestrictionStore() : BlockRestrictionStore { + return MediaWikiServices::getInstance()->getBlockRestrictionStore(); + } } diff --git a/tests/phpunit/includes/block/BlockRestrictionStoreTest.php b/tests/phpunit/includes/block/BlockRestrictionStoreTest.php new file mode 100644 index 0000000000..4eef457339 --- /dev/null +++ b/tests/phpunit/includes/block/BlockRestrictionStoreTest.php @@ -0,0 +1,612 @@ +blockRestrictionStore = MediaWikiServices::getInstance()->getBlockRestrictionStore(); + } + + public function tearDown() { + parent::tearDown(); + $this->resetTables(); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testLoadMultipleRestrictions() { + $this->setMwGlobals( [ + 'wgBlockDisablesLogin' => false, + ] ); + $block = $this->insertBlock(); + + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $pageFoo->getId() ), + new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ), + ] ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + + $this->assertCount( 3, $restrictions ); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testWithNoRestrictions() { + $block = $this->insertBlock(); + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertEmpty( $restrictions ); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testWithEmptyParam() { + $restrictions = $this->blockRestrictionStore->loadByBlockId( [] ); + $this->assertEmpty( $restrictions ); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testIgnoreNotSupportedTypes() { + $block = $this->insertBlock(); + + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + + // valid type + $this->insertRestriction( $block->getId(), PageRestriction::TYPE_ID, $pageFoo->getId() ); + $this->insertRestriction( $block->getId(), NamespaceRestriction::TYPE_ID, NS_USER ); + + // invalid type + $this->insertRestriction( $block->getId(), 9, $pageBar->getId() ); + $this->insertRestriction( $block->getId(), 10, NS_FILE ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 2, $restrictions ); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testMappingPageRestrictionObject() { + $block = $this->insertBlock(); + $title = 'Lady Macbeth'; + $page = $this->getExistingTestPage( $title ); + + // Test Page Restrictions. + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + + list( $pageRestriction ) = $restrictions; + $this->assertInstanceOf( PageRestriction::class, $pageRestriction ); + $this->assertEquals( $block->getId(), $pageRestriction->getBlockId() ); + $this->assertEquals( $page->getId(), $pageRestriction->getValue() ); + $this->assertEquals( $pageRestriction->getType(), PageRestriction::TYPE ); + $this->assertEquals( $pageRestriction->getTitle()->getText(), $title ); + } + + /** + * @covers ::loadByBlockId + * @covers ::resultToRestrictions + * @covers ::rowToRestriction + */ + public function testMappingNamespaceRestrictionObject() { + $block = $this->insertBlock(); + + $this->blockRestrictionStore->insert( [ + new NamespaceRestriction( $block->getId(), NS_USER ), + ] ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + + list( $namespaceRestriction ) = $restrictions; + $this->assertInstanceOf( NamespaceRestriction::class, $namespaceRestriction ); + $this->assertEquals( $block->getId(), $namespaceRestriction->getBlockId() ); + $this->assertSame( NS_USER, $namespaceRestriction->getValue() ); + $this->assertEquals( $namespaceRestriction->getType(), NamespaceRestriction::TYPE ); + } + + /** + * @covers ::insert + */ + public function testInsert() { + $block = $this->insertBlock(); + + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + + $restrictions = [ + new \stdClass(), + new PageRestriction( $block->getId(), $pageFoo->getId() ), + new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ) + ]; + + $result = $this->blockRestrictionStore->insert( $restrictions ); + $this->assertTrue( $result ); + + $restrictions = [ + new \stdClass(), + ]; + + $result = $this->blockRestrictionStore->insert( $restrictions ); + $this->assertFalse( $result ); + + $result = $this->blockRestrictionStore->insert( [] ); + $this->assertFalse( $result ); + } + + /** + * @covers ::insert + */ + public function testInsertTypes() { + $block = $this->insertBlock(); + + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + + $invalid = $this->createMock( Restriction::class ); + $invalid->method( 'toRow' ) + ->willReturn( [ + 'ir_ipb_id' => $block->getId(), + 'ir_type' => 9, + 'ir_value' => 42, + ] ); + + $restrictions = [ + new \stdClass(), + new PageRestriction( $block->getId(), $pageFoo->getId() ), + new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ), + $invalid, + ]; + + $result = $this->blockRestrictionStore->insert( $restrictions ); + $this->assertTrue( $result ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 3, $restrictions ); + } + + /** + * @covers ::update + * @covers ::restrictionsByBlockId + * @covers ::restrictionsToRemove + */ + public function testUpdateInsert() { + $block = $this->insertBlock(); + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $pageFoo->getId() ), + ] ); + + $this->blockRestrictionStore->update( [ + new \stdClass(), + new PageRestriction( $block->getId(), $pageBar->getId() ), + new NamespaceRestriction( $block->getId(), NS_USER ), + ] ); + + $db = wfGetDb( DB_REPLICA ); + $result = $db->select( + [ 'ipblocks_restrictions' ], + [ '*' ], + [ 'ir_ipb_id' => $block->getId() ] + ); + + $this->assertEquals( 2, $result->numRows() ); + $row = $result->fetchObject(); + $this->assertEquals( $block->getId(), $row->ir_ipb_id ); + $this->assertEquals( $pageBar->getId(), $row->ir_value ); + } + + /** + * @covers ::update + * @covers ::restrictionsByBlockId + * @covers ::restrictionsToRemove + */ + public function testUpdateChange() { + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + + $this->blockRestrictionStore->update( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $db = wfGetDb( DB_REPLICA ); + $result = $db->select( + [ 'ipblocks_restrictions' ], + [ '*' ], + [ 'ir_ipb_id' => $block->getId() ] + ); + + $this->assertEquals( 1, $result->numRows() ); + $row = $result->fetchObject(); + $this->assertEquals( $block->getId(), $row->ir_ipb_id ); + $this->assertEquals( $page->getId(), $row->ir_value ); + } + + /** + * @covers ::update + * @covers ::restrictionsByBlockId + * @covers ::restrictionsToRemove + */ + public function testUpdateNoRestrictions() { + $block = $this->insertBlock(); + + $this->blockRestrictionStore->update( [] ); + + $db = wfGetDb( DB_REPLICA ); + $result = $db->select( + [ 'ipblocks_restrictions' ], + [ '*' ], + [ 'ir_ipb_id' => $block->getId() ] + ); + + $this->assertEquals( 0, $result->numRows() ); + } + + /** + * @covers ::update + * @covers ::restrictionsByBlockId + * @covers ::restrictionsToRemove + */ + public function testUpdateSame() { + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $this->blockRestrictionStore->update( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $db = wfGetDb( DB_REPLICA ); + $result = $db->select( + [ 'ipblocks_restrictions' ], + [ '*' ], + [ 'ir_ipb_id' => $block->getId() ] + ); + + $this->assertEquals( 1, $result->numRows() ); + $row = $result->fetchObject(); + $this->assertEquals( $block->getId(), $row->ir_ipb_id ); + $this->assertEquals( $page->getId(), $row->ir_value ); + } + + /** + * @covers ::updateByParentBlockId + */ + public function testDeleteAllUpdateByParentBlockId() { + // Create a block and an autoblock (a child block) + $block = $this->insertBlock(); + $pageFoo = $this->getExistingTestPage( 'Foo' ); + $pageBar = $this->getExistingTestPage( 'Bar' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $pageFoo->getId() ), + ] ); + $autoblockId = $block->doAutoblock( '127.0.0.1' ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); + + // Ensure that the restrictions on the autoblock are the same as the block. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 1, $restrictions ); + $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); + + // Update the restrictions on the autoblock (but leave the block unchanged) + $this->blockRestrictionStore->updateByParentBlockId( $block->getId(), [ + new PageRestriction( $block->getId(), $pageBar->getId() ), + ] ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); + + // Ensure that the restrictions on the autoblock have been updated. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 1, $restrictions ); + $this->assertEquals( $pageBar->getId(), $restrictions[0]->getValue() ); + } + + /** + * @covers ::updateByParentBlockId + */ + public function testUpdateByParentBlockId() { + // Create a block and an autoblock (a child block) + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + $autoblockId = $block->doAutoblock( '127.0.0.1' ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + // Ensure that the restrictions on the autoblock have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 1, $restrictions ); + + // Remove the restrictions on the autoblock (but leave the block unchanged) + $this->blockRestrictionStore->updateByParentBlockId( $block->getId(), [] ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + // Ensure that the restrictions on the autoblock have been updated. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 0, $restrictions ); + } + + /** + * @covers ::updateByParentBlockId + */ + public function testNoAutoblocksUpdateByParentBlockId() { + // Create a block with no autoblock. + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + // Update the restrictions on any autoblocks (there are none). + $this->blockRestrictionStore->updateByParentBlockId( $block->getId(), $restrictions ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + } + + /** + * @covers ::delete + */ + public function testDelete() { + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + $result = $this->blockRestrictionStore->delete( + array_merge( $restrictions, [ new \stdClass() ] ) + ); + $this->assertTrue( $result ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 0, $restrictions ); + } + + /** + * @covers ::deleteByBlockId + */ + public function testDeleteByBlockId() { + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + $result = $this->blockRestrictionStore->deleteByBlockId( $block->getId() ); + $this->assertNotFalse( $result ); + + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 0, $restrictions ); + } + + /** + * @covers ::deleteByParentBlockId + */ + public function testDeleteByParentBlockId() { + // Create a block with no autoblock. + $block = $this->insertBlock(); + $page = $this->getExistingTestPage( 'Foo' ); + $this->blockRestrictionStore->insert( [ + new PageRestriction( $block->getId(), $page->getId() ), + ] ); + $autoblockId = $block->doAutoblock( '127.0.0.1' ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + // Ensure that the restrictions on the autoblock are the same as the block. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 1, $restrictions ); + + // Remove all of the restrictions on the autoblock (but leave the block unchanged). + $result = $this->blockRestrictionStore->deleteByParentBlockId( $block->getId() ); + // NOTE: commented out until https://gerrit.wikimedia.org/r/c/mediawiki/core/+/469324 is merged + //$this->assertTrue( $result ); + + // Ensure that the restrictions on the block have not changed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $block->getId() ); + $this->assertCount( 1, $restrictions ); + + // Ensure that the restrictions on the autoblock have been removed. + $restrictions = $this->blockRestrictionStore->loadByBlockId( $autoblockId ); + $this->assertCount( 0, $restrictions ); + } + + /** + * @covers ::equals + * @dataProvider equalsDataProvider + * + * @param array $a + * @param array $b + * @param bool $expected + */ + public function testEquals( array $a, array $b, $expected ) { + $this->assertSame( $expected, $this->blockRestrictionStore->equals( $a, $b ) ); + } + + public function equalsDataProvider() { + return [ + [ + [ + new \stdClass(), + new PageRestriction( 1, 1 ), + ], + [ + new \stdClass(), + new PageRestriction( 1, 2 ) + ], + false, + ], + [ + [ + new PageRestriction( 1, 1 ), + ], + [ + new PageRestriction( 1, 1 ), + new PageRestriction( 1, 2 ) + ], + false, + ], + [ + [], + [], + true, + ], + [ + [ + new PageRestriction( 1, 1 ), + new PageRestriction( 1, 2 ), + new PageRestriction( 2, 3 ), + ], + [ + new PageRestriction( 2, 3 ), + new PageRestriction( 1, 2 ), + new PageRestriction( 1, 1 ), + ], + true + ], + [ + [ + new NamespaceRestriction( 1, NS_USER ), + ], + [ + new NamespaceRestriction( 1, NS_USER ), + ], + true + ], + [ + [ + new NamespaceRestriction( 1, NS_USER ), + ], + [ + new NamespaceRestriction( 1, NS_TALK ), + ], + false + ], + ]; + } + + /** + * @covers ::setBlockId + */ + public function testSetBlockId() { + $restrictions = [ + new \stdClass(), + new PageRestriction( 1, 1 ), + new PageRestriction( 1, 2 ), + new NamespaceRestriction( 1, NS_USER ), + ]; + + $this->assertSame( 1, $restrictions[1]->getBlockId() ); + $this->assertSame( 1, $restrictions[2]->getBlockId() ); + $this->assertSame( 1, $restrictions[3]->getBlockId() ); + + $result = $this->blockRestrictionStore->setBlockId( 2, $restrictions ); + + foreach ( $result as $restriction ) { + $this->assertSame( 2, $restriction->getBlockId() ); + } + } + + protected function insertBlock() { + $badActor = $this->getTestUser()->getUser(); + $sysop = $this->getTestSysop()->getUser(); + + $block = new \Block( [ + 'address' => $badActor->getName(), + 'user' => $badActor->getId(), + 'by' => $sysop->getId(), + 'expiry' => 'infinity', + 'sitewide' => 0, + 'enableAutoblock' => true, + ] ); + + $block->insert(); + + return $block; + } + + protected function insertRestriction( $blockId, $type, $value ) { + $this->db->insert( 'ipblocks_restrictions', [ + 'ir_ipb_id' => $blockId, + 'ir_type' => $type, + 'ir_value' => $value, + ] ); + } + + protected function resetTables() { + $this->db->delete( 'ipblocks', '*', __METHOD__ ); + $this->db->delete( 'ipblocks_restrictions', '*', __METHOD__ ); + } +} diff --git a/tests/phpunit/includes/block/BlockRestrictionTest.php b/tests/phpunit/includes/block/BlockRestrictionTest.php deleted file mode 100644 index 5bbd3d023d..0000000000 --- a/tests/phpunit/includes/block/BlockRestrictionTest.php +++ /dev/null @@ -1,601 +0,0 @@ -resetTables(); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testLoadMultipleRestrictions() { - $this->setMwGlobals( [ - 'wgBlockDisablesLogin' => false, - ] ); - $block = $this->insertBlock(); - - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $pageFoo->getId() ), - new PageRestriction( $block->getId(), $pageBar->getId() ), - new NamespaceRestriction( $block->getId(), NS_USER ), - ] ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - - $this->assertCount( 3, $restrictions ); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testWithNoRestrictions() { - $block = $this->insertBlock(); - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertEmpty( $restrictions ); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testWithEmptyParam() { - $restrictions = BlockRestriction::loadByBlockId( [] ); - - $this->assertEmpty( $restrictions ); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testIgnoreNotSupportedTypes() { - $block = $this->insertBlock(); - - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - - // valid type - $this->insertRestriction( $block->getId(), PageRestriction::TYPE_ID, $pageFoo->getId() ); - $this->insertRestriction( $block->getId(), NamespaceRestriction::TYPE_ID, NS_USER ); - - // invalid type - $this->insertRestriction( $block->getId(), 9, $pageBar->getId() ); - $this->insertRestriction( $block->getId(), 10, NS_FILE ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 2, $restrictions ); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testMappingPageRestrictionObject() { - $block = $this->insertBlock(); - $title = 'Lady Macbeth'; - $page = $this->getExistingTestPage( $title ); - - // Test Page Restrictions. - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - - list( $pageRestriction ) = $restrictions; - $this->assertInstanceOf( PageRestriction::class, $pageRestriction ); - $this->assertEquals( $block->getId(), $pageRestriction->getBlockId() ); - $this->assertEquals( $page->getId(), $pageRestriction->getValue() ); - $this->assertEquals( $pageRestriction->getType(), PageRestriction::TYPE ); - $this->assertEquals( $pageRestriction->getTitle()->getText(), $title ); - } - - /** - * @covers ::loadByBlockId - * @covers ::resultToRestrictions - * @covers ::rowToRestriction - */ - public function testMappingNamespaceRestrictionObject() { - $block = $this->insertBlock(); - - BlockRestriction::insert( [ - new NamespaceRestriction( $block->getId(), NS_USER ), - ] ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - - list( $namespaceRestriction ) = $restrictions; - $this->assertInstanceOf( NamespaceRestriction::class, $namespaceRestriction ); - $this->assertEquals( $block->getId(), $namespaceRestriction->getBlockId() ); - $this->assertSame( NS_USER, $namespaceRestriction->getValue() ); - $this->assertEquals( $namespaceRestriction->getType(), NamespaceRestriction::TYPE ); - } - - /** - * @covers ::insert - */ - public function testInsert() { - $block = $this->insertBlock(); - - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - - $restrictions = [ - new \stdClass(), - new PageRestriction( $block->getId(), $pageFoo->getId() ), - new PageRestriction( $block->getId(), $pageBar->getId() ), - new NamespaceRestriction( $block->getId(), NS_USER ) - ]; - - $result = BlockRestriction::insert( $restrictions ); - $this->assertTrue( $result ); - - $restrictions = [ - new \stdClass(), - ]; - - $result = BlockRestriction::insert( $restrictions ); - $this->assertFalse( $result ); - - $result = BlockRestriction::insert( [] ); - $this->assertFalse( $result ); - } - - /** - * @covers ::insert - */ - public function testInsertTypes() { - $block = $this->insertBlock(); - - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - - $invalid = $this->createMock( Restriction::class ); - $invalid->method( 'toRow' ) - ->willReturn( [ - 'ir_ipb_id' => $block->getId(), - 'ir_type' => 9, - 'ir_value' => 42, - ] ); - - $restrictions = [ - new \stdClass(), - new PageRestriction( $block->getId(), $pageFoo->getId() ), - new PageRestriction( $block->getId(), $pageBar->getId() ), - new NamespaceRestriction( $block->getId(), NS_USER ), - $invalid, - ]; - - $result = BlockRestriction::insert( $restrictions ); - $this->assertTrue( $result ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 3, $restrictions ); - } - - /** - * @covers ::update - * @covers ::restrictionsByBlockId - * @covers ::restrictionsToRemove - */ - public function testUpdateInsert() { - $block = $this->insertBlock(); - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $pageFoo->getId() ), - ] ); - - BlockRestriction::update( [ - new \stdClass(), - new PageRestriction( $block->getId(), $pageBar->getId() ), - new NamespaceRestriction( $block->getId(), NS_USER ), - ] ); - - $db = wfGetDb( DB_REPLICA ); - $result = $db->select( - [ 'ipblocks_restrictions' ], - [ '*' ], - [ 'ir_ipb_id' => $block->getId() ] - ); - - $this->assertEquals( 2, $result->numRows() ); - $row = $result->fetchObject(); - $this->assertEquals( $block->getId(), $row->ir_ipb_id ); - $this->assertEquals( $pageBar->getId(), $row->ir_value ); - } - - /** - * @covers ::update - * @covers ::restrictionsByBlockId - * @covers ::restrictionsToRemove - */ - public function testUpdateChange() { - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - - BlockRestriction::update( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - $db = wfGetDb( DB_REPLICA ); - $result = $db->select( - [ 'ipblocks_restrictions' ], - [ '*' ], - [ 'ir_ipb_id' => $block->getId() ] - ); - - $this->assertEquals( 1, $result->numRows() ); - $row = $result->fetchObject(); - $this->assertEquals( $block->getId(), $row->ir_ipb_id ); - $this->assertEquals( $page->getId(), $row->ir_value ); - } - - /** - * @covers ::update - * @covers ::restrictionsByBlockId - * @covers ::restrictionsToRemove - */ - public function testUpdateNoRestrictions() { - $block = $this->insertBlock(); - - BlockRestriction::update( [] ); - - $db = wfGetDb( DB_REPLICA ); - $result = $db->select( - [ 'ipblocks_restrictions' ], - [ '*' ], - [ 'ir_ipb_id' => $block->getId() ] - ); - - $this->assertEquals( 0, $result->numRows() ); - } - - /** - * @covers ::update - * @covers ::restrictionsByBlockId - * @covers ::restrictionsToRemove - */ - public function testUpdateSame() { - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - BlockRestriction::update( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - $db = wfGetDb( DB_REPLICA ); - $result = $db->select( - [ 'ipblocks_restrictions' ], - [ '*' ], - [ 'ir_ipb_id' => $block->getId() ] - ); - - $this->assertEquals( 1, $result->numRows() ); - $row = $result->fetchObject(); - $this->assertEquals( $block->getId(), $row->ir_ipb_id ); - $this->assertEquals( $page->getId(), $row->ir_value ); - } - - /** - * @covers ::updateByParentBlockId - */ - public function testDeleteAllUpdateByParentBlockId() { - // Create a block and an autoblock (a child block) - $block = $this->insertBlock(); - $pageFoo = $this->getExistingTestPage( 'Foo' ); - $pageBar = $this->getExistingTestPage( 'Bar' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $pageFoo->getId() ), - ] ); - $autoblockId = $block->doAutoblock( '127.0.0.1' ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); - - // Ensure that the restrictions on the autoblock are the same as the block. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 1, $restrictions ); - $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); - - // Update the restrictions on the autoblock (but leave the block unchanged) - BlockRestriction::updateByParentBlockId( $block->getId(), [ - new PageRestriction( $block->getId(), $pageBar->getId() ), - ] ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - $this->assertEquals( $pageFoo->getId(), $restrictions[0]->getValue() ); - - // Ensure that the restrictions on the autoblock have been updated. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 1, $restrictions ); - $this->assertEquals( $pageBar->getId(), $restrictions[0]->getValue() ); - } - - /** - * @covers ::updateByParentBlockId - */ - public function testUpdateByParentBlockId() { - // Create a block and an autoblock (a child block) - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - $autoblockId = $block->doAutoblock( '127.0.0.1' ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - // Ensure that the restrictions on the autoblock have not changed. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 1, $restrictions ); - - // Remove the restrictions on the autoblock (but leave the block unchanged) - BlockRestriction::updateByParentBlockId( $block->getId(), [] ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - // Ensure that the restrictions on the autoblock have been updated. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 0, $restrictions ); - } - - /** - * @covers ::updateByParentBlockId - */ - public function testNoAutoblocksUpdateByParentBlockId() { - // Create a block with no autoblock. - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - // Update the restrictions on any autoblocks (there are none). - BlockRestriction::updateByParentBlockId( $block->getId(), $restrictions ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - } - - /** - * @covers ::delete - */ - public function testDelete() { - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - $result = BlockRestriction::delete( array_merge( $restrictions, [ new \stdClass() ] ) ); - $this->assertTrue( $result ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 0, $restrictions ); - } - - /** - * @covers ::deleteByBlockId - */ - public function testDeleteByBlockId() { - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - $result = BlockRestriction::deleteByBlockId( $block->getId() ); - $this->assertNotFalse( $result ); - - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 0, $restrictions ); - } - - /** - * @covers ::deleteByParentBlockId - */ - public function testDeleteByParentBlockId() { - // Create a block with no autoblock. - $block = $this->insertBlock(); - $page = $this->getExistingTestPage( 'Foo' ); - BlockRestriction::insert( [ - new PageRestriction( $block->getId(), $page->getId() ), - ] ); - $autoblockId = $block->doAutoblock( '127.0.0.1' ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - // Ensure that the restrictions on the autoblock are the same as the block. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 1, $restrictions ); - - // Remove all of the restrictions on the autoblock (but leave the block unchanged). - $result = BlockRestriction::deleteByParentBlockId( $block->getId() ); - // NOTE: commented out until https://gerrit.wikimedia.org/r/c/mediawiki/core/+/469324 is merged - //$this->assertTrue( $result ); - - // Ensure that the restrictions on the block have not changed. - $restrictions = BlockRestriction::loadByBlockId( $block->getId() ); - $this->assertCount( 1, $restrictions ); - - // Ensure that the restrictions on the autoblock have been removed. - $restrictions = BlockRestriction::loadByBlockId( $autoblockId ); - $this->assertCount( 0, $restrictions ); - } - - /** - * @covers ::equals - * @dataProvider equalsDataProvider - * - * @param array $a - * @param array $b - * @param bool $expected - */ - public function testEquals( array $a, array $b, $expected ) { - $this->assertSame( $expected, BlockRestriction::equals( $a, $b ) ); - } - - public function equalsDataProvider() { - return [ - [ - [ - new \stdClass(), - new PageRestriction( 1, 1 ), - ], - [ - new \stdClass(), - new PageRestriction( 1, 2 ) - ], - false, - ], - [ - [ - new PageRestriction( 1, 1 ), - ], - [ - new PageRestriction( 1, 1 ), - new PageRestriction( 1, 2 ) - ], - false, - ], - [ - [], - [], - true, - ], - [ - [ - new PageRestriction( 1, 1 ), - new PageRestriction( 1, 2 ), - new PageRestriction( 2, 3 ), - ], - [ - new PageRestriction( 2, 3 ), - new PageRestriction( 1, 2 ), - new PageRestriction( 1, 1 ), - ], - true - ], - [ - [ - new NamespaceRestriction( 1, NS_USER ), - ], - [ - new NamespaceRestriction( 1, NS_USER ), - ], - true - ], - [ - [ - new NamespaceRestriction( 1, NS_USER ), - ], - [ - new NamespaceRestriction( 1, NS_TALK ), - ], - false - ], - ]; - } - - /** - * @covers ::setBlockId - */ - public function testSetBlockId() { - $restrictions = [ - new \stdClass(), - new PageRestriction( 1, 1 ), - new PageRestriction( 1, 2 ), - new NamespaceRestriction( 1, NS_USER ), - ]; - - $this->assertSame( 1, $restrictions[1]->getBlockId() ); - $this->assertSame( 1, $restrictions[2]->getBlockId() ); - $this->assertSame( 1, $restrictions[3]->getBlockId() ); - - $result = BlockRestriction::setBlockId( 2, $restrictions ); - - foreach ( $result as $restriction ) { - $this->assertSame( 2, $restriction->getBlockId() ); - } - } - - protected function insertBlock() { - $badActor = $this->getTestUser()->getUser(); - $sysop = $this->getTestSysop()->getUser(); - - $block = new \Block( [ - 'address' => $badActor->getName(), - 'user' => $badActor->getId(), - 'by' => $sysop->getId(), - 'expiry' => 'infinity', - 'sitewide' => 0, - 'enableAutoblock' => true, - ] ); - - $block->insert(); - - return $block; - } - - protected function insertRestriction( $blockId, $type, $value ) { - $this->db->insert( 'ipblocks_restrictions', [ - 'ir_ipb_id' => $blockId, - 'ir_type' => $type, - 'ir_value' => $value, - ] ); - } - - protected function resetTables() { - $this->db->delete( 'ipblocks', '*', __METHOD__ ); - $this->db->delete( 'ipblocks_restrictions', '*', __METHOD__ ); - } -} diff --git a/tests/phpunit/includes/specials/SpecialBlockTest.php b/tests/phpunit/includes/specials/SpecialBlockTest.php index 182ca0d043..c1f2e42064 100644 --- a/tests/phpunit/includes/specials/SpecialBlockTest.php +++ b/tests/phpunit/includes/specials/SpecialBlockTest.php @@ -1,9 +1,10 @@ assertSame( $reason, $block->getReason() ); $this->assertSame( $expiry, $block->getExpiry() ); $this->assertCount( 2, $block->getRestrictions() ); - $this->assertTrue( BlockRestriction::equals( $block->getRestrictions(), [ + $this->assertTrue( $this->getBlockRestrictionStore()->equals( $block->getRestrictions(), [ new PageRestriction( $block->getId(), $pageMars->getId() ), new PageRestriction( $block->getId(), $pageSaturn->getId() ), ] ) ); @@ -335,7 +336,7 @@ class SpecialBlockTest extends SpecialPageTestBase { $this->assertSame( $expiry, $block->getExpiry() ); $this->assertFalse( $block->isSitewide() ); $this->assertCount( 2, $block->getRestrictions() ); - $this->assertTrue( BlockRestriction::equals( $block->getRestrictions(), [ + $this->assertTrue( $this->getBlockRestrictionStore()->equals( $block->getRestrictions(), [ new PageRestriction( $block->getId(), $pageMars->getId() ), new PageRestriction( $block->getId(), $pageSaturn->getId() ), ] ) ); @@ -351,7 +352,7 @@ class SpecialBlockTest extends SpecialPageTestBase { $this->assertSame( $expiry, $block->getExpiry() ); $this->assertFalse( $block->isSitewide() ); $this->assertCount( 1, $block->getRestrictions() ); - $this->assertTrue( BlockRestriction::equals( $block->getRestrictions(), [ + $this->assertTrue( $this->getBlockRestrictionStore()->equals( $block->getRestrictions(), [ new PageRestriction( $block->getId(), $pageMars->getId() ), ] ) ); @@ -471,4 +472,17 @@ class SpecialBlockTest extends SpecialPageTestBase { $this->db->delete( 'ipblocks', '*', __METHOD__ ); $this->db->delete( 'ipblocks_restrictions', '*', __METHOD__ ); } + + /** + * Get a BlockRestrictionStore instance + * + * @return BlockRestrictionStore + */ + private function getBlockRestrictionStore() : BlockRestrictionStore { + $loadBalancer = $this->getMockBuilder( LoadBalancer::class ) + ->disableOriginalConstructor() + ->getMock(); + + return new BlockRestrictionStore( $loadBalancer ); + } }